From 747a26bd8ec02575bc5f41feddc17875c7feb131 Mon Sep 17 00:00:00 2001 From: robertlipe Date: Wed, 20 Feb 2013 05:56:01 +0000 Subject: [PATCH] Make a serious attack on the new XML serializer for GPX. This version actually totally passes our test suite, byte for byte, even when our old version was kind of dumb. There's still plenty of work to be done but it's at a point where we could consider turning it on by default soon. --- gpsbabel/garmin_fs.cc | 66 +++++++++++++++++- gpsbabel/garmin_fs.h | 3 +- gpsbabel/gpx.cc | 154 ++++++++++++++++++++++++++++-------------- 3 files changed, 169 insertions(+), 54 deletions(-) diff --git a/gpsbabel/garmin_fs.cc b/gpsbabel/garmin_fs.cc index e52c48d0b..58dd64b51 100644 --- a/gpsbabel/garmin_fs.cc +++ b/gpsbabel/garmin_fs.cc @@ -25,6 +25,8 @@ #include "garmin_tables.h" #include "inifile.h" +#include + #define MYNAME "garmin_fs" #define GARMIN_GPX_EXT_REFERENCE \ @@ -183,7 +185,8 @@ void garmin_fs_convert(void* fs) /* GPX - out */ void -garmin_fs_xml_fprint(gbfile* ofd, const waypoint* waypt) +garmin_fs_xml_fprint(gbfile* ofd, const waypoint* waypt, + QXmlStreamWriter& writer) { const char* phone, *addr; garmin_fs_t* gmsd = GMSD_FIND(waypt); @@ -216,9 +219,26 @@ garmin_fs_xml_fprint(gbfile* ofd, const waypoint* waypt) WAYPT_HAS(waypt, temperature) || gmsd->flags.display) { int space = 1; - +#if OLDGPX gbfprintf(ofd, "%*s\n", space++ * 2, ""); gbfprintf(ofd, "%*s\n", space++ * 2, "", GARMIN_GPX_EXT_REFERENCE); +#else + writer.writeStartElement("extensions"); + writer.writeStartElement("gpxx:WaypointExtension"); + writer.writeNamespace("http://www.garmin.com/xmlschemas/GpxExtensions/v3", + "gpxx"); + writer.writeNamespace("http://www.w3.org/2001/XMLSchema-instance", + "xsi"); + writer.writeAttribute("xsi:schemaLocation", + "http://www.garmin.com/xmlschemas/GpxExtensions/v3 " + "http://www.garmin.com/xmlschemas/GpxExtensions/v3/GpxExtensionsv3.xsd"); +// "http://www.garmin.com/xmlschemas/GpxExtensions/v3/GpxExtensionsv3.xsd" +// "xmlns:gpxx=\"" \ +// "xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" " \ +// "xsi:schemaLocation=\"http://www.garmin.com/xmlschemas/GpxExtensions/v3 " \ +// "http://www.garmin.com/xmlschemas/GpxExtensions/v3/GpxExtensionsv3.xsd" +// writer.writeEndElement(); +#endif if WAYPT_HAS(waypt, proximity) { gbfprintf(ofd, "%*s%.6f\n", space * 2, "", waypt->proximity); } @@ -241,7 +261,11 @@ garmin_fs_xml_fprint(gbfile* ofd, const waypoint* waypt) cx = "SymbolAndName"; break; } +#if OLDGPX gbfprintf(ofd, "%*s%s\n", space * 2, "", cx); +#else + writer.writeTextElement("gpxx:DisplayMode", cx); +#endif } if (gmsd->flags.category && gmsd->category) { int i; @@ -257,45 +281,81 @@ garmin_fs_xml_fprint(gbfile* ofd, const waypoint* waypt) } if (*addr) { char* str, *tmp; +#if OLDGPX gbfprintf(ofd, "%*s\n", space++ * 2, ""); +#else + writer.writeStartElement("gpxx:Address"); +#endif if ((str = GMSD_GET(addr, NULL))) { +#if OLDGPX tmp = xml_entitize(str); gbfprintf(ofd, "%*s%s\n", space * 2, "", tmp); xfree(tmp); +#else + writer.writeTextElement("gpxx:StreetAddress", str); +#endif } if ((str = GMSD_GET(city, NULL))) { +#if OLDGPX tmp = xml_entitize(str); gbfprintf(ofd, "%*s%s\n", space * 2, "", tmp); xfree(tmp); +#else + writer.writeTextElement("gpxx:City", str); +#endif } if ((str = GMSD_GET(state, NULL))) { +#if OLDGPX tmp = xml_entitize(str); gbfprintf(ofd, "%*s%s\n", space * 2, "", tmp); xfree(tmp); +#else + writer.writeTextElement("gpxx:State", str); +#endif } if ((str = GMSD_GET(country, NULL))) { +#if OLDGPX tmp = xml_entitize(str); gbfprintf(ofd, "%*s%s\n", space * 2, "", tmp); +#else + writer.writeTextElement("gpxx:Country", str); +#endif xfree(tmp); } if ((str = GMSD_GET(postal_code, NULL))) { +#if OLDGPX tmp = xml_entitize(str); gbfprintf(ofd, "%*s%s\n", space * 2, "", tmp); xfree(tmp); +#else + writer.writeTextElement("gpxx:PostalCode", str); +#endif } - +#if OLDGPX gbfprintf(ofd, "%*s\n", --space * 2, ""); +#else + writer.writeEndElement(); // /gpxx::Address +#endif } if (*phone) { +#if OLDGPX char* tmp = xml_entitize(phone); gbfprintf(ofd, "%*s%s\n", space * 2, "", tmp); xfree(tmp); +#else + writer.writeTextElement("gpxx:PhoneNumber", phone); +#endif } +#if OLDGPX gbfprintf(ofd, "%*s\n", --space * 2, ""); gbfprintf(ofd, "%*s\n", --space * 2, ""); +#else + writer.writeEndElement(); // /gpxx::WaypointExtension + writer.writeEndElement(); // /extensions. +#endif } } diff --git a/gpsbabel/garmin_fs.h b/gpsbabel/garmin_fs.h index c3f994d0e..a050c21af 100644 --- a/gpsbabel/garmin_fs.h +++ b/gpsbabel/garmin_fs.h @@ -122,7 +122,8 @@ char* garmin_fs_xstrdup(const char* src, size_t size); /* for GPX */ void garmin_fs_xml_convert(const int base_tag, int tag, const char* cdatastr, waypoint* waypt); -void garmin_fs_xml_fprint(gbfile* ofd, const waypoint* waypt); +class QXmlStreamWriter; +void garmin_fs_xml_fprint(gbfile* ofd, const waypoint* waypt, QXmlStreamWriter&); /* common garmin_fs utilities */ diff --git a/gpsbabel/gpx.cc b/gpsbabel/gpx.cc index a6fc548eb..2188e843c 100644 --- a/gpsbabel/gpx.cc +++ b/gpsbabel/gpx.cc @@ -1,4 +1,3 @@ -#define OLD 1 // Crutch while bringing up new XML writer /* Access GPX data files. @@ -30,8 +29,9 @@ static XML_Parser psr; #endif #include +#include #include -#include +//#include static xml_tag* cur_tag; @@ -279,14 +279,14 @@ gpx_write_gdata(gpx_global_entry* ge, const char* tag) if (!gpx_global || QUEUE_EMPTY(&ge->queue)) { return; } -#if OLD +#if OLDGPX gbfprintf(ofd, "<%s>", tag); #else writer.writeStartElement(tag); #endif QUEUE_FOR_EACH(&ge->queue, elem, tmp) { gep = BASE_STRUCT(elem, gpx_global_entry, queue); -#if OLD +#if OLDGPX gbfprintf(ofd, "%s", gep->tagdata); #else writer.writeCharacters(gep->tagdata); @@ -298,7 +298,7 @@ gpx_write_gdata(gpx_global_entry* ge, const char* tag) } gbfprintf(ofd, " "); } -#if OLD +#if OLDGPX gbfprintf(ofd, "\n", tag); #else writer.writeEndElement(); @@ -1398,7 +1398,7 @@ gpx_wr_init(const char* fname) static void gpx_wr_deinit(void) { -#if OLD +#if OLDGPX gbfclose(ofd); #else writer.writeEndDocument(); @@ -1411,6 +1411,13 @@ gpx_wr_deinit(void) // for now, in the early days, just to keep things as we've always done it. ostring.replace("'", "'"); + // TODO: The old writer would more aggressively protect you from control + // character nonsense. The control-Z (032)is the only thing that appears in + // our test suite, but let's toss things we know aren't allowed in GPX. + // Let's just carpet-bomb the whole range for now. I have a feeling we'll + // revisit this in time... + ostring.replace(QRegExp("[\014-\032]"), " "); + gbfputs(ostring, ofd); gbfclose(ofd); ofd = NULL; @@ -1519,13 +1526,13 @@ static void fprint_tag_and_attrs(const char* prefix, const char* suffix, xml_tag* tag) { char** pa; -#if OLD +#if OLDGPX gbfprintf(ofd, "%s%s", prefix, tag->tagname); #endif pa = tag->attributes; if (pa) { while (*pa) { -#if OLD +#if OLDGPX gbfprintf(ofd, " %s=\"%s\"", pa[0], pa[1]); pa += 2; #else @@ -1533,7 +1540,7 @@ fprint_tag_and_attrs(const char* prefix, const char* suffix, xml_tag* tag) #endif } } -#if OLD +#if OLDGPX gbfprintf(ofd, "%s", suffix); #else // writer.writeEndElement(); @@ -1551,7 +1558,7 @@ fprint_xml_chain(xml_tag* tag, const waypoint* wpt) fprint_tag_and_attrs("<", ">", tag); if (tag->cdata) { -#if OLD +#if OLDGPX char* tmp_ent; tmp_ent = xml_entitize(tag->cdata); gbfprintf(ofd, "%s", tmp_ent); @@ -1568,7 +1575,7 @@ fprint_xml_chain(xml_tag* tag, const waypoint* wpt) xml_write_time(ofd, wpt->gc_data->exported, 0, "groundspeak:exported"); } -#if OLD +#if OLDGPX gbfprintf(ofd, "\n", tag->tagname); #else writer.writeEndElement(); @@ -1634,7 +1641,7 @@ write_gpx_url(const waypoint* waypointp) return; } -#if OLD +#if OLDGPX char* tmp_ent; if (gpx_wversion_num > 10) { url_link* tail; @@ -1664,8 +1671,12 @@ write_gpx_url(const waypoint* waypointp) writer.writeAttribute("href", tail->url); } if(tail->url_link_text && tail->url_link_text[0]) { - writer.writeAttribute("text", tail->url_link_text); + writer.writeTextElement("text", tail->url_link_text); } + // FIXME This is to force empty links to not be self-closing. This is + // lame, but it's for compatibilty with our old writer to minimize thrash + // on the Qt transition. + writer.writeCharacters("\n"); writer.writeEndElement(); } return; @@ -1708,7 +1719,7 @@ gpx_write_common_acc(const waypoint* waypointp, const char* indent) default: break; } -#if OLD +#if OLDGPX if (fix) { gbfprintf(ofd, "%s%s\n", indent, fix); } @@ -1746,7 +1757,7 @@ static void gpx_write_common_position(const waypoint* waypointp, const char* indent) { if (waypointp->altitude != unknown_alt) { -#if OLD +#if OLDGPX gbfprintf(ofd, "%s%f\n", indent, waypointp->altitude); #else @@ -1754,11 +1765,12 @@ gpx_write_common_position(const waypoint* waypointp, const char* indent) #endif } if (waypointp->creation_time) { -#if OLD +#if OLDGPX gbfprintf(ofd, indent); xml_write_time(ofd, waypointp->creation_time, waypointp->microseconds, "time"); #else char time_string[64]; + // FIXME: Eventually use creation_time.toString() xml_fill_in_time(time_string, waypointp->creation_time, waypointp->microseconds, XML_LONG_TIME); if (time_string[0]) { writer.writeTextElement("time", time_string); @@ -1772,6 +1784,7 @@ gpx_write_common_extensions(const waypoint* waypointp, const char* indent) { if (((opt_humminbirdext || opt_garminext) && (waypointp->depth != 0 || waypointp->temperature != 0)) || (opt_garminext && (waypointp->heartrate != 0 || waypointp->cadence != 0))) { +#if OLDGPX gbfprintf(ofd, "%s\n", indent); if (waypointp->depth != 0) { if (opt_humminbirdext) @@ -1800,6 +1813,43 @@ gpx_write_common_extensions(const waypoint* waypointp, const char* indent) gbfprintf(ofd, "%s \n", indent); } gbfprintf(ofd, "%s\n", indent); +#else + writer.writeStartElement("extensions"); + if (waypointp->depth != 0) { + if (opt_humminbirdext) { +// gbfprintf(ofd, "%s %f\n", +// indent, waypointp->depth*100.0); +writer.writeTextElement("h:depth", QString::number(waypointp->depth * 100.0)); + } + if (opt_garminext) { +#if OLDGPX + gbfprintf(ofd, "%s %f\n", + indent, waypointp->depth); +#else +writer.writeTextElement("gpxx", "Depth", QString::number(waypointp->depth)); +#endif + } + } + if (waypointp->temperature != 0) { + if (opt_humminbirdext) + gbfprintf(ofd, "%s %f\n", + indent, waypointp->temperature); + if (opt_garminext) + gbfprintf(ofd, "%s %f\n", + indent, waypointp->temperature); + } + if (opt_garminext && (waypointp->heartrate != 0 || waypointp->cadence != 0)) { + gbfprintf(ofd, "%s \n", indent); + if (waypointp->heartrate != 0) + gbfprintf(ofd, "%s %u\n", + indent, waypointp->heartrate); + if (waypointp->cadence != 0) + gbfprintf(ofd, "%s %u\n", + indent, waypointp->cadence); + gbfprintf(ofd, "%s \n", indent); + } + writer.writeEndElement(); // "extensions" +#endif } } @@ -1807,7 +1857,7 @@ static void gpx_write_common_description(const waypoint* waypointp, const char* indent, const char* oname) { -#if OLD +#if OLDGPX write_optional_xml_entity(ofd, indent, "name", oname); write_optional_xml_entity(ofd, indent, "cmt", waypointp->description); if (waypointp->notes && waypointp->notes[0]) { @@ -1844,7 +1894,7 @@ gpx_waypt_pr(const waypoint* waypointp) const char* oname; fs_xml* fs_gpx; garmin_fs_t* gmsd; /* gARmIN sPECIAL dATA */ -#if OLD +#if OLDGPX gbfprintf(ofd, "\n", waypointp->latitude, waypointp->longitude); @@ -1869,10 +1919,10 @@ gpx_waypt_pr(const waypoint* waypointp) } if (gmsd && (gpx_wversion_num > 10)) { /* MapSource doesn't accepts extensions from 1.0 */ - garmin_fs_xml_fprint(ofd, waypointp); + garmin_fs_xml_fprint(ofd, waypointp, writer); } gpx_write_common_extensions(waypointp, " "); -#if OLD +#if OLDGPX gbfprintf(ofd, "\n"); #else writer.writeEndElement(); @@ -1884,7 +1934,7 @@ gpx_track_hdr(const route_head* rte) { fs_xml* fs_gpx; current_trk_head = rte; -#if OLD +#if OLDGPX gbfprintf(ofd, "\n"); write_optional_xml_entity(ofd, " ", "name", rte->rte_name); write_optional_xml_entity(ofd, " ", "desc", rte->rte_desc); @@ -1923,19 +1973,19 @@ gpx_track_disp(const waypoint* waypointp) if (waypointp->wpt_flags.new_trkseg) { if (!first_in_trk) { -#if OLD +#if OLDGPX gbfprintf(ofd, "\n"); #else writer.writeEndElement(); #endif } -#if OLD +#if OLDGPX gbfprintf(ofd, "\n"); #else writer.writeStartElement("trkseg"); #endif } -#if OLD +#if OLDGPX gbfprintf(ofd, "\n", waypointp->latitude, waypointp->longitude); @@ -1958,7 +2008,7 @@ gpx_track_disp(const waypoint* waypointp) /* These were accidentally removed from 1.1 */ if (gpx_wversion_num == 10) { if WAYPT_HAS(waypointp, course) { -#if OLD +#if OLDGPX gbfprintf(ofd, " %f\n", waypointp->course); #else @@ -1966,7 +2016,7 @@ gpx_track_disp(const waypoint* waypointp) #endif } if WAYPT_HAS(waypointp, speed) { -#if OLD +#if OLDGPX gbfprintf(ofd, " %f\n", waypointp->speed); #else @@ -1992,7 +2042,7 @@ gpx_track_disp(const waypoint* waypointp) } gpx_write_common_extensions(waypointp, " "); -#if OLD +#if OLDGPX gbfprintf(ofd, "\n"); #else writer.writeEndElement(); @@ -2003,13 +2053,13 @@ static void gpx_track_tlr(const route_head* rte) { if (!QUEUE_EMPTY(¤t_trk_head->waypoint_list)) { -#if OLD +#if OLDGPX gbfprintf(ofd, "\n"); #else writer.writeEndElement(); #endif } -#if OLD +#if OLDGPX gbfprintf(ofd, "\n"); #else // FIXME This is to force empty tracks to not be self-closing. This is @@ -2031,24 +2081,24 @@ static void gpx_route_hdr(const route_head* rte) { fs_xml* fs_gpx; -#if OLD +#if OLDGPX gbfprintf(ofd, "\n"); #else writer.writeStartElement("rte"); #endif -#if OLD +#if OLDGPX write_optional_xml_entity(ofd, " ", "name", rte->rte_name); write_optional_xml_entity(ofd, " ", "desc", rte->rte_desc); #else - if (rte->rte_name) { + if (rte->rte_name && rte->rte_name[0]) { writer.writeTextElement("name", rte->rte_name); } - if (rte->rte_desc) { + if (rte->rte_desc && rte->rte_desc[0]) { writer.writeTextElement("desc", rte->rte_desc); } #endif if (rte->rte_num) { -#if OLD +#if OLDGPX gbfprintf(ofd, " %d\n", rte->rte_num); #else writer.writeTextElement("number", QString::number(rte->rte_num)); @@ -2068,7 +2118,7 @@ gpx_route_disp(const waypoint* waypointp) { const char* oname; fs_xml* fs_gpx; -#if OLD +#if OLDGPX gbfprintf(ofd, " \n", waypointp->latitude, waypointp->longitude); @@ -2091,13 +2141,9 @@ gpx_route_disp(const waypoint* waypointp) } gpx_write_common_extensions(waypointp, " "); -#if OLD +#if OLDGPX gbfprintf(ofd, " \n"); #else - // FIXME This is to force empty tracks to not be self-closing. This is - // lame, but it's for compatibilty with our old writer to minimize thrash - // on the Qt transition. - writer.writeCharacters("\n"); writer.writeEndElement(); #endif } @@ -2105,9 +2151,13 @@ gpx_route_disp(const waypoint* waypointp) static void gpx_route_tlr(const route_head* rte) { -#if OLD +#if OLDGPX gbfprintf(ofd, "\n"); #else + // FIXME This is to force empty tracks to not be self-closing. This is + // lame, but it's for compatibilty with our old writer to minimize thrash + // on the Qt transition. + writer.writeCharacters("\n"); writer.writeEndElement(); // Close rte tag. #endif } @@ -2135,7 +2185,7 @@ gpx_write_bounds(void) track_disp_all(NULL, NULL, gpx_waypt_bound_calc); if (waypt_bounds_valid(&all_bounds)) { -#if OLD +#if OLDGPX gbfprintf(ofd, "\n", all_bounds.min_lat, all_bounds.min_lon, @@ -2179,7 +2229,7 @@ gpx_write(void) } now = current_time(); -#if OLD +#if OLDGPX gbfprintf(ofd, "\n", global_opts.charset_name); gbfprintf(ofd, "\n", xsi_schema_loc); #else writer.writeAttribute("\n xsi:schemaLocation", xsi_schema_loc); #endif } else { -#if OLD +#if OLDGPX gbfprintf(ofd, " xsi:schemaLocation=" DEFAULT_XSI_SCHEMA_LOC_FMT">\n", gpx_wversion[0], gpx_wversion[2], @@ -2222,7 +2276,7 @@ gpx_write(void) } if (gpx_wversion_num > 10) { -#if OLD +#if OLDGPX gbfprintf(ofd, "\n"); #else writer.writeStartElement("metadata"); @@ -2242,7 +2296,7 @@ gpx_write(void) gpx_write_gdata(&gpx_global->url, "url"); gpx_write_gdata(&gpx_global->urlname, "urlname"); } -#if OLD +#if OLDGPX xml_write_time(ofd, now, 0, "time"); #else char time_string[64]; @@ -2256,7 +2310,7 @@ gpx_write(void) gpx_write_bounds(); if (gpx_wversion_num > 10) { -#if OLD +#if OLDGPX gbfprintf(ofd, "\n"); #else writer.writeEndElement(); @@ -2269,7 +2323,7 @@ gpx_write(void) gpx_route_pr(); gpx_reset_short_handle(); gpx_track_pr(); -#if OLD +#if OLDGPX gbfprintf(ofd, "\n"); #else writer.writeEndElement(); // Close gpx tag. -- 2.30.2